!!! FEATURE: References on creation and Copy #5148

kitsunet · 2024-06-17T16:42:23Z

This reworks references so that multiple reference properties can be set via a single command and also references can be attached to CreateNodeAggregateWithNode which is also used for copying nodes.

The serialization format is adapted to allow multiple reference properties, which also affects all behat tests with references.

To migrate existing events run:

./flow migrateevents:migratesetreferencestomultinameformat

Marked breaking as it requires an event migration.

This requires neos/neos-ui#3844 for the UI

A behat test to show reference copies work was added.

Upgrade Instructions

The API to set references changed a little, please adjust accordingly.
This is a simple example which covers all cases:

SetNodeReferences::create(
    $node->workspaceName,
    $node->aggregateId,
    $node->originDimensionSpacePoint,
    NodeReferencesToWrite::create(
        // sets the 3 nodes as references
        NodeReferencesForName::fromTargets(
            ReferenceName::fromString('first-reference'),
            NodeAggregateIds::fromArray(['node-aggregate-id-1', 'node-aggregate-id-2', 'node-aggregate-id-3'])
        ),
        // unset previously set references (as we do NOT merge the values)
        NodeReferencesForName::createEmpty(
            ReferenceName::fromString('second-reference')
        ),
        // create a simple node reference and additionally a reference with an additional property
        NodeReferencesForName::fromReferences(
            ReferenceName::fromString('third-reference'),
            [
                NodeReferenceToWrite::fromTarget(NodeAggregateId::fromString('node-aggregate-id-6')),
                NodeReferenceToWrite::fromTargetAndProperties(NodeAggregateId::fromString('node-aggregate-id-5'), PropertyValuesToWrite::fromArray([
                    'propertyOnReference' => 'Hi im living on the edge ;)'
                ])),
            ])
        )
    )
)

kitsunet · 2024-06-17T16:43:21Z

Do not merge yet, probably want to refactor some of the code in the DTOs. I think we should make the "collection of references for one reference property" an explicit DTO, as the code like this feels a bit too messy for my taste...

kitsunet · 2024-06-17T16:45:53Z

On the upside the somewhat weird and unfinished reference snapshots can be removed with this.

kitsunet · 2024-06-17T16:51:51Z

And forgot to adapt legacy migration tests (which rely on specific event payloads, see below, before I adjust those we should decide which way to go with b/c)...

kitsunet · 2024-06-17T16:54:50Z

oh and I should mention that at the moment this is not forgiving to the existing events. We could do this in the respective fromArray constructors of the DTOs and just "upcast" from the old structure on the fly without much problem, or add a migration for the events.

mhsdesign

Thanks for tackling this, some early thoughts before ill play with this ;)

Neos.ContentRepository.Core/Classes/Feature/Common/NodeReferencingInternals.php

Neos.ContentRepository.Core/Classes/Feature/NodeReferencing/Dto/NodeReferenceNameToEmpty.php

Neos.ContentRepository.Core/Classes/Feature/NodeReferencing/Dto/NodeReferenceToWrite.php

Neos.ContentRepository.Core/Classes/Feature/NodeReferencing/Dto/NodeReferencesToWrite.php

Neos.ContentRepository.Core/Classes/Feature/NodeReferencing/Dto/NodeReferenceToWrite.php

kitsunet · 2024-06-18T06:48:41Z

I think this would be much nicer code wise (obviously duplicated for Serialized...), bit annoying to use 3 objects for this, but I think it will make the code much better to reason with and empty is then really just the new "middle DTO" with createEmpty()

This reworks references so that multiple reference properties can be set via a single command and also references can be attached to `CreateNodeAggregateWithNode` which is also used for copying nodes.

# Conflicts: # Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphProjection.php

mhsdesign · 2024-10-12T06:24:36Z

I remember we discussed this some time ago so i was wondering where it goes from now? I think we discussed for an alternative way than to use a "hacky" NodeReferenceNameToEmpty?

…-copy

mhsdesign

ill re review later:)

Neos.ContentRepositoryRegistry/Classes/Service/EventMigrationService.php

...Tests/Tests/Behavior/Features/05-NodeReferencing/04-SetNodeReferences_PropertyScopes.feature

...Repository.BehavioralTests/Tests/Behavior/Features/NodeCopying/CopyNode_NoDimensions.feature

Neos.ContentRepository.Core/Classes/Feature/Common/NodeReferencingInternals.php

....ContentRepository.Core/Classes/Feature/NodeCreation/Command/CreateNodeAggregateWithNode.php

.../Classes/Feature/NodeCreation/Command/CreateNodeAggregateWithNodeAndSerializedProperties.php

Neos.ContentRepository.Core/Classes/Feature/NodeDuplication/Dto/NodeReferenceSnapshot.php

...ntentRepository.Core/Classes/Feature/NodeReferencing/Dto/SerializedNodeReferencesForName.php

Neos.ContentRepository.Core/Classes/Feature/NodeReferencing/Dto/SerializedNodeReference.php

Neos.ContentRepositoryRegistry/Classes/Service/EventMigrationService.php

Neos.ContentRepository.LegacyNodeMigration/Classes/NodeDataToEventsProcessor.php

Neos.ContentRepositoryRegistry/Classes/Service/EventMigrationService.php

…places

…`fromReferences`

…sted

… override previous values

…aggregate ids to write for ONE reference name

bwaidelich

Great work.. and a lot of it, yikes.
Some rather minor issues

Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphProjection.php

bwaidelich · 2024-10-31T16:02:15Z

Neos.ContentGraph.PostgreSQLAdapter/src/Domain/Projection/Feature/NodeReferencing.php

+                    $this->getDatabaseConnection()->delete($this->tableNamePrefix . '_referencerelation', [
+                        'sourcenodeanchor' => $anchorPoint->value,
+                        'name' => $referencesForProperty->referenceName->value
+                    ]);
+
+                    foreach ($referencesForProperty->references as $reference) {
+                        // set new
+                        $referenceRecord = new ReferenceRelationRecord(
+                            $anchorPoint,
+                            $referencesForProperty->referenceName,
+                            $position,
+                            $reference->properties,
+                            $reference->targetNodeAggregateId
+                        );
+                        $referenceRecord->addToDatabase($this->getDatabaseConnection(), $this->tableNamePrefix);
+                        $position++;
+                    }


Same here, can't we turn this into two atomic queries?

for me the pg adapter is out of scope and there are no tests ... but i fixed it for doctrine

Neos.ContentRepository.Core/Classes/Feature/NodeDuplication/Dto/NodeSubtreeSnapshot.php

Neos.ContentRepository.Core/Classes/Feature/NodeReferencing/Dto/NodeReferencesForName.php

bwaidelich · 2024-10-31T16:14:07Z

Neos.ContentRepository.Core/Classes/Feature/NodeReferencing/NodeReferencing.php

+            $this->requireNodeTypeToDeclareReference($sourceNodeAggregate->nodeTypeName, $referenceName);
+            $scopeDeclaration = $sourceNodeType->getReferences()[$referenceName->value]['scope'] ?? '';
+            $scope = PropertyScope::tryFrom($scopeDeclaration) ?: PropertyScope::SCOPE_NODE;
+            // TODO: Optimize affected sets into one event


Is there a specific reason why we don't?
The current state would be like publishing multiple NodePropertiesWereSet events for each property.

Couldn't we just build up the $referencesByName array and then build the event outside of the loop?

Neos.ContentRepositoryRegistry/Classes/Service/EventMigrationService.php

mhsdesign · 2024-10-31T16:55:04Z

and the test has to be adjusted as it fails now earlier:

001 Scenario: Node reference cannot hold multiple targets to the same node                                          # Features/05-NodeReferencing/01-SetNodeReferences_ConstraintChecks.feature:190
      Then the last command should have thrown an exception of type "InvalidArgumentException" with code 1700150910 # Features/05-NodeReferencing/01-SetNodeReferences_ConstraintChecks.feature:195
        Expected exception code 1700150910, got exception code 1730365958 instead; Message: Duplicate entry in references to write. Target "anthony-destinode" already exists in collection.
        Failed asserting that 1730365958 is identical to 1700150910.

…-copy

and remove duplicated `fromReferences` naming in chain, now its `create` for root level

… list

mhsdesign · 2024-10-31T20:31:37Z

Okay i fixed the tests again and added some documentation ... i also got rid of the duplicate NodeReferencesToWrite::fromReferences and its now just create and also the fromNameAnd parts are out for the NodeReferencesForName as well its guaranteed to stick to a name and thus makes it a little shorter and hopefully easy to use.

SetNodeReferences::create(
    $node->workspaceName,
    $node->aggregateId,
    $node->originDimensionSpacePoint,
    NodeReferencesToWrite::create(
        // sets the 3 nodes as references
        NodeReferencesForName::fromTargets(
            ReferenceName::fromString('first-reference'),
            NodeAggregateIds::fromArray(['node-aggregate-id-1', 'node-aggregate-id-2', 'node-aggregate-id-3'])
        ),
        // unset previously set references (as we do NOT merge the values)
        NodeReferencesForName::createEmpty(
            ReferenceName::fromString('second-reference')
        ),
        // create a simple node reference and additionally a reference with an additional property
        NodeReferencesForName::fromReferences(
            ReferenceName::fromString('third-reference'),
            [
                NodeReferenceToWrite::fromTarget(NodeAggregateId::fromString('node-aggregate-id-6')),
                NodeReferenceToWrite::fromTargetAndProperties(NodeAggregateId::fromString('node-aggregate-id-5'), PropertyValuesToWrite::fromArray([
                    'propertyOnReference' => 'Hi im living on the edge ;)'
                ])),
            ])
    )
)

... like in set serialized properties. That way fewer `NodeReferencesWereSet` are emitted. At most only one per scope of `PropertyScope` Also the `PropertyScope` enum was marked internal as its not exposed in apis and no use for the user.

mhsdesign

Okay i further made the optimisations basti suggested. Now its truly finished id say <3

Thank you so much for your effort!

github-actions bot added Feature 9.0 labels Jun 17, 2024

mhsdesign reviewed Jun 17, 2024

View reviewed changes

FEATURE: References on creation and Copy

1612659

This reworks references so that multiple reference properties can be set via a single command and also references can be attached to `CreateNodeAggregateWithNode` which is also used for copying nodes.

kitsunet force-pushed the feature/node-reference-copy branch from 1ee4c27 to 1612659 Compare August 23, 2024 12:20

kitsunet added 7 commits August 26, 2024 12:50

Fix merge errors and syntax problems

c20c17b

More syntax fixes

c50fcb7

Remove forgotten debug statement

8c40cbb

Fix new unit Tests

b600c70

Fix another new unit test

72d5f61

Adjust new behat test with References

7519353

Fix broken export test

1bc6e38

kitsunet requested a review from nezaniel August 27, 2024 11:14

kitsunet mentioned this pull request Aug 30, 2024

Adjustments to new Reference DTOs neos/neos-ui#3844

Merged

Merge branch '9.0' into feature/node-reference-copy

a9ae648

# Conflicts: # Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphProjection.php

kitsunet added 5 commits October 21, 2024 10:26

Merge remote-tracking branch 'origin/9.0' into feature/node-reference…

a4cc38c

…-copy

Cleaner References write DTOs

80b719e

Empty properties in refs without special treatment

be16480

Merge branch '9.0' into feature/node-reference-copy

5505b5e

Cleanup

a74c9b3

kitsunet requested review from mhsdesign and bwaidelich October 23, 2024 07:18

Add event migration for references

98dead5

kitsunet changed the title ~~FEATURE: References on creation and Copy~~ !!! FEATURE: References on creation and Copy Oct 23, 2024

Merge remote-tracking branch 'origin/9.0' into feature/node-reference…

da531d1

…-copy

mhsdesign reviewed Oct 28, 2024

View reviewed changes

mhsdesign mentioned this pull request Oct 30, 2024

Setup / Migration #5245

Open

4 tasks

kitsunet and others added 3 commits October 30, 2024 19:03

Code cleanup

1ffad19

Code cleanup

f0bb466

TASK: Fix migrateReferencesToMultiFormat

27e9550

kitsunet commented Oct 30, 2024

View reviewed changes

Neos.ContentRepositoryRegistry/Classes/Service/EventMigrationService.php Show resolved Hide resolved

kitsunet commented Oct 30, 2024

View reviewed changes

Neos.ContentRepositoryRegistry/Classes/Service/EventMigrationService.php Show resolved Hide resolved

mhsdesign added 8 commits October 30, 2024 20:44

TASK: Simplify code by using automatic jsonSerialize like in other …

a6ff4a1

…places

TASK: Use variadics to verify array list shape

0c18918

TASK: Introduce fromNameAndTargets utility and accept flat list in …

9694c97

…`fromReferences`

TASK: Remove obsolet serialisation logic from vo's that are not persi…

826b47d

…sted

TASK: Introduce NodeReferencesToWrite::withReference and make merge…

a28c27d

… override previous values

TASK: Validate NodeReferencesForName to not contain duplicate node …

48efbea

…aggregate ids to write for ONE reference name

TASK: Simplify SerializedNodeReferences capabilities

a8cfb25

TASK: inline fromReadReferences so its @internal

d254397

bwaidelich reviewed Oct 31, 2024

View reviewed changes

mhsdesign added 4 commits October 31, 2024 20:37

Merge remote-tracking branch 'origin/9.0' into feature/node-reference…

b800c66

…-copy

TASK: Adjust documentation and namings

7cadf90

TASK: Adjust namings to remove obsolete fromNameAnd part

3310d74

and remove duplicated `fromReferences` naming in chain, now its `create` for root level

TASK: Ensure that SerializedNodeReferencesForName are serialized as…

4eee438

… list

mhsdesign added 2 commits October 31, 2024 21:46

TASK: Make atomic delete instead of deleting old references one by one

bc2427c

mhsdesign approved these changes Oct 31, 2024

View reviewed changes

kitsunet enabled auto-merge November 1, 2024 10:17

kitsunet disabled auto-merge November 1, 2024 10:17

kitsunet merged commit 4d00a25 into neos:9.0 Nov 1, 2024
10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

!!! FEATURE: References on creation and Copy #5148

!!! FEATURE: References on creation and Copy #5148

kitsunet commented Jun 17, 2024 •

edited by mhsdesign

Loading

kitsunet commented Jun 17, 2024

kitsunet commented Jun 17, 2024 •

edited

Loading

kitsunet commented Jun 17, 2024 •

edited

Loading

kitsunet commented Jun 17, 2024

mhsdesign left a comment

kitsunet commented Jun 18, 2024

mhsdesign commented Oct 12, 2024

mhsdesign left a comment

bwaidelich left a comment

bwaidelich Oct 31, 2024

mhsdesign Oct 31, 2024

bwaidelich Oct 31, 2024

mhsdesign Oct 31, 2024

mhsdesign commented Oct 31, 2024

mhsdesign commented Oct 31, 2024

mhsdesign left a comment

!!! FEATURE: References on creation and Copy #5148

!!! FEATURE: References on creation and Copy #5148

Conversation

kitsunet commented Jun 17, 2024 • edited by mhsdesign Loading

kitsunet commented Jun 17, 2024

kitsunet commented Jun 17, 2024 • edited Loading

kitsunet commented Jun 17, 2024 • edited Loading

kitsunet commented Jun 17, 2024

mhsdesign left a comment

Choose a reason for hiding this comment

kitsunet commented Jun 18, 2024

mhsdesign commented Oct 12, 2024

mhsdesign left a comment

Choose a reason for hiding this comment

bwaidelich left a comment

Choose a reason for hiding this comment

bwaidelich Oct 31, 2024

Choose a reason for hiding this comment

mhsdesign Oct 31, 2024

Choose a reason for hiding this comment

bwaidelich Oct 31, 2024

Choose a reason for hiding this comment

mhsdesign Oct 31, 2024

Choose a reason for hiding this comment

mhsdesign commented Oct 31, 2024

mhsdesign commented Oct 31, 2024

mhsdesign left a comment

Choose a reason for hiding this comment

kitsunet commented Jun 17, 2024 •

edited by mhsdesign

Loading

kitsunet commented Jun 17, 2024 •

edited

Loading

kitsunet commented Jun 17, 2024 •

edited

Loading